general fixes to adapt table model #417
Conversation
* [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 23.10.1 → 23.11.0](psf/black@23.10.1...23.11.0) - [github.com/pre-commit/mirrors-prettier: v3.0.3 → v3.1.0](pre-commit/mirrors-prettier@v3.0.3...v3.1.0) - [github.com/pre-commit/mirrors-mypy: v1.6.1 → v1.7.0](pre-commit/mirrors-mypy@v1.6.1...v1.7.0) - [github.com/astral-sh/ruff-pre-commit: v0.1.3 → v0.1.6](astral-sh/ruff-pre-commit@v0.1.3...v0.1.6) * ficx pre-precommit --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: giovp <[email protected]>
for more information, see https://pre-commit.ci
…into multi_table
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…into multi_table
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…into multi_table
Co-authored-by: Wouter-Michiel Vierdag <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…into multi_table
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…into multi_table
for more information, see https://pre-commit.ci
|
In the other PR there is currently 1 remaining errors which relates to removing the |
|
@melonora can you please check if this branch can merged/removed? |
| @@ -672,7 +671,7 @@ def filter_by_coordinate_system( | |||
| # TODO: check whether full table dict should be returned or only those which annotate elements. Also check | |||
| # filtering with tables having potentially different keys. | |||
| if filter_tables: | |||
| tables = {} | |||
| tables: Tables = Tables(shared_keys=self._shared_keys) | |||
There was a problem hiding this comment.
comparing this to #410 there is a difference here. However, I am not certain that shared_keys needs to be passed here since we are returning a new SpatialData object. What do you think @giovp @LucaMarconato ?
There was a problem hiding this comment.
yeah I agree, the shared keys should be instantiated in the return line 680. I think this can be closed!
There was a problem hiding this comment.
After offline conversation with Giovanni, we agreed it is not required.
|
I will close this PR as all the changes have already been implemented in #410 |
@melonora these are some additional fixes, I think we need to remove the add_table/store_table etc. due to the merge of #329 and keep the Tables element as close as possible to the rest.